Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse MySQL column default value #110

Merged
merged 18 commits into from
Jun 10, 2023
Merged

Parse MySQL column default value #110

merged 18 commits into from
Jun 10, 2023

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 commented Jun 7, 2023

PR Info

Bug Fixes

  • Parse MySQL column default value

@billy1624 billy1624 marked this pull request as ready for review June 7, 2023 10:40
@billy1624 billy1624 requested a review from tyt2y3 June 7, 2023 11:34
default[1..(default.len() - 1)].into(),
))
} else if default == "NULL" {
None
Copy link
Member

@tyt2y3 tyt2y3 Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 'default NULL' has a different meaning to 'no default'.
Seems like we should add a new variant ColumnDefault::Null.

In practice, the default value could be 0 or '' instead of NULL.

@@ -264,7 +264,25 @@ pub fn parse_column_default(column_default: Option<String>) -> Option<ColumnDefa
match column_default {
Some(default) => {
if !default.is_empty() {
Copy link
Member

@tyt2y3 tyt2y3 Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just read https://dev.mysql.com/doc/refman/8.0/en/data-type-defaults.html again,
I think the logic should be as follows:

  1. parse the EXTRA column; if it contains DEFAULT_GENERATED, then the COLUMN_DEFAULT should be regarded as an expression
  2. if the default is an expression, parse it as a Expr and recognize the keywords CURRENT_TIMESTAMP, otherwise Custom
  3. if the default is not an expression, then it's a literal; then we should
    a. if it is numeric-like then regard it as Number
    b. otherwise regard it as String

image
image

1. if matches a keyword (CURRENT_TIMESTAMP) then make it a corresponding keyword
  1. if it starts with a ( regard it as an expression
  2. if it is numeric-like then regard it as Number. not sure whether it's worth the effort to classify into int and real/decimal
  3. finally, consider it a String

It's really a legacy that MySQL does not quote a literal string. It's given as hello instead of 'hello'.

Edit: I was wrong; apparently COLUMNS table does not give a () for expressions either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we cannot match the ( character. The expression (CURRENT_TIMESTAMP) or CURRENT_TIMESTAMP both are valid syntax to define default value on table creation. But when you query the default expression via information schema, you get different result.

image

@billy1624
Copy link
Member Author

billy1624 commented Jun 8, 2023

  1. parse the EXTRA column; if it contains DEFAULT_GENERATED, then the COLUMN_DEFAULT should be regarded as an expression

Hmmm... no. This logic doesn't work on every MySQL / Mariadb version

image

@tyt2y3
Copy link
Member

tyt2y3 commented Jun 8, 2023

May be we simply regard CURRENT_TIMESTAMP as special, but only if the column type is timestamp or datatime.
Note that, CURRENT_DATE is not special.

Before MariaDB 10.2.1 you couldn't usually provide an expression or function to evaluate at insertion time. You had to provide a constant default value instead. The one exception is that you may use CURRENT_TIMESTAMP as the default value for a TIMESTAMP column to use the current timestamp at insertion time.
CURRENT_TIMESTAMP may also be used as the default value for a DATETIME
From MariaDB 10.2.1 you can use most functions in DEFAULT. Expressions should have parentheses around them

and this https://mariadb.com/kb/en/information-schema-columns-table/ https://jira.mariadb.org/browse/MDEV-13132
apparently MariaDB would sometimes quote the literal

Default value for the column. From MariaDB 10.2.7, literals are quoted to distinguish them from expressions. NULL means that the column has no default. In MariaDB 10.2.6 and earlier, no quotes were used for any type of default and NULL can either mean that there is no default, or that the default column value is NULL.

@tyt2y3
Copy link
Member

tyt2y3 commented Jun 8, 2023

Sadly to implement this correctly we have to perform some version check:

MySQL 5:
Only literal is supported, so:

  1. if column is TIMESTAMP or DATETIME and value is CURRENT_TIMESTAMP, regard it as DefaultExpr::CurrentTimestamp
  2. if it is numeric-like then regard it as Number
  3. otherwise regard it as String

MySQL 8:

  1. parse the EXTRA column; if it contains DEFAULT_GENERATED, then the COLUMN_DEFAULT should be regarded as an expression
  2. if the default is an expression, parse it as a Expr and recognize the keywords CURRENT_TIMESTAMP, otherwise Custom
  3. if the default is not an expression, then it's a literal; then we should
    a. if it is numeric-like then regard it as Number
    b. otherwise regard it as String

MariaDB < 10.2.1:
Same as MySQL 5

MariaDB >= 10.2.1:

  1. if it starts with a ' it must be a String
  2. if it is numeric-like then it is a Number
  3. otherwise it is an expression, and can be one of the keywords (CURRENT_TIMESTAMP)

@billy1624
Copy link
Member Author

Hey @tyt2y3, was a success. Please proofread the db specific parsing part for me :)

@billy1624 billy1624 requested a review from tyt2y3 June 8, 2023 14:16
Comment on lines 295 to 298
if is_date_time_col && default == "CURRENT_DATE" {
ColumnDefault::CurrentDate
} else if is_date_time_col && default == "CURRENT_TIME" {
ColumnDefault::CurrentTime
Copy link
Member

@tyt2y3 tyt2y3 Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty promising! My only question is, are you sure current_date/time is 'as special as' current_timestamp?

Judging by the docs, it only mentioned CURRENT_TIMESTAMP:

The exception is that, for TIMESTAMP and DATETIME columns, you can specify the CURRENT_TIMESTAMP function as the default, without enclosing parentheses. See Section 11.2.5, “Automatic Initialization and Updating for TIMESTAMP and DATETIME”.

It didn't say CURRENT_DATE can be used for DATE, etc.

This means, for example, that you cannot set the default for a date column to be the value of a function such as NOW() or CURRENT_DATE.

I belive it does not work under MySQL 5.x

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just double checked, yes, you're right. current_date/time cannot be used as default value / expression.

Copy link
Member Author

@billy1624 billy1624 Jun 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed CurrentDate and CurrentTime fca3ee5

}

pub fn parse_mysql_8_default(default: String, extra: &str) -> ColumnDefault {
let is_expression = extra == "DEFAULT_GENERATED";
Copy link
Member

@tyt2y3 tyt2y3 Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra can have more string parts, not just DEFAULT_GENERATED, a contains() will be safer.
The right way would be to split the string by ' ' and then Vec::contains.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else if let Ok(double) = default.parse::<f64>() {
ColumnDefault::Double(double)
} else {
ColumnDefault::String(default)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case the tail case should be split:

  1. is_expression = true -> ColumnDefault::CustomExpr
  2. is_expression = false -> ColumnDefault::String

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else if default == "NULL" {
ColumnDefault::Null
} else {
ColumnDefault::String(default)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the final case is a CustomExpr;
https://mariadb.com/kb/en/create-table/#default-column-option

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@billy1624 billy1624 requested a review from tyt2y3 June 9, 2023 07:28
@billy1624
Copy link
Member Author

Done

@tyt2y3
Copy link
Member

tyt2y3 commented Jun 10, 2023

I just checked on my machine, MySQL 8 default (NULL) is valid and is different from default null

@tyt2y3
Copy link
Member

tyt2y3 commented Jun 10, 2023

Wait... I am re-reading this

From MariaDB 10.2.7, literals are quoted to distinguish them from expressions. NULL means that the column has no default

It means for the versions >= 10.2.1 and < 10.2.7 is a void, and there is no way to properly parse the default.

@tyt2y3 tyt2y3 merged commit 1cd22b5 into master Jun 10, 2023
@tyt2y3 tyt2y3 deleted the parse-col-default-val branch June 10, 2023 18:01
@github-actions
Copy link

🎉 Released In 0.12.0 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fail to distinguish MySQL default expression / value
2 participants